Skip to content

dcd/musb: defer EP0 SETUP during DATA_IN/STATUS race#3643

Open
HiFiPhile wants to merge 2 commits into
masterfrom
musb_ep0_race
Open

dcd/musb: defer EP0 SETUP during DATA_IN/STATUS race#3643
HiFiPhile wants to merge 2 commits into
masterfrom
musb_ep0_race

Conversation

@HiFiPhile
Copy link
Copy Markdown
Collaborator

@HiFiPhile HiFiPhile commented May 14, 2026

Handle cases where a new SETUP arrives before the previous control transfer fully completes by buffering the SETUP and replaying it after status completion. Split EP0 DATA state into DATA_IN/DATA_OUT and finalize pending status-out completion before processing deferred SETUP.

Take care these 2 cases if new SETUP packet arrived while old control transfer is not finished yet. This could happen in following scenarios when CPU load is high:

  • Status IN/OUT finished, IRQ and new setup packet IRQ arrive at the same time.
  • Data IN finished and status OUT is received, both IRQs and new setup packet IRQ arrive at the same time.

Supercede #3626

Tested intensively with DFU example with Interrupt turned on/off every 20ms simulating high CP load:

diff --git a/examples/device/dfu/src/main.c b/examples/device/dfu/src/main.c
index fb3c22630..d4e75a44b 100644
--- a/examples/device/dfu/src/main.c
+++ b/examples/device/dfu/src/main.c
@@ -45,6 +45,7 @@
 #include "bsp/board_api.h"
 #include "tusb.h"
 
+#include "device/dcd.h"
 //--------------------------------------------------------------------+
 // MACRO CONSTANT TYPEDEF PROTYPES
 //--------------------------------------------------------------------+
@@ -65,6 +66,7 @@ enum {
 static uint32_t blink_interval_ms = BLINK_NOT_MOUNTED;
 
 void led_blinking_task(void);
+static void musb_test(void);
 
 /*------------- MAIN -------------*/
 int main(void) {
@@ -79,6 +81,7 @@ int main(void) {
   while (1) {
     tud_task(); // tinyusb device task
     led_blinking_task();
+    musb_test();
   }
 }
 
@@ -86,6 +89,26 @@ int main(void) {
 // Device callbacks
 //--------------------------------------------------------------------+
 
+// on off usb interrupt 20ms for testing
+static void musb_test(void) {
+  static uint32_t start_ms  = 0;
+  static bool irq_enabled = true;
+
+  if (tusb_time_millis_api() - start_ms < 20) {
+    return; // not enough time
+  }
+
+  if (irq_enabled) {
+    dcd_int_disable(0);
+    tusb_time_delay_ms_api(20);
+    dcd_int_enable(0);
+  } else {
+  }
+
+  irq_enabled = !irq_enabled;
+  start_ms += 20;
+}
+
 // Invoked when device is mounted
 void tud_mount_cb(void) {
   blink_interval_ms = BLINK_MOUNTED;
@@ -122,7 +145,7 @@ uint32_t tud_dfu_get_timeout_cb(uint8_t alt, uint8_t state) {
     // For this example
     // - Atl0 Flash is fast : 1   ms
     // - Alt1 EEPROM is slow: 100 ms
-    return (alt == 0) ? 1 : 100;
+    return (alt == 0) ? 0 : 0;
   } else if (state == DFU_MANIFEST) {
     // since we don't buffer entire image and do any flashing in manifest stage
     return 0;
@@ -142,9 +165,9 @@ void tud_dfu_download_cb(uint8_t alt, uint16_t block_num, const uint8_t *data, u
 
   //printf("\r\nReceived Alt %u BlockNum %u of length %u\r\n", alt, wBlockNum, length);
 
-  for (uint16_t i = 0; i < length; i++) {
-    printf("%c", data[i]);
-  }
+  //for (uint16_t i = 0; i < length; i++) {
+  //  printf("%c", data[i]);
+  //}
 
   // flashing op for download complete without error
   tud_dfu_finish_flashing(DFU_STATUS_OK);

Copilot AI review requested due to automatic review settings May 14, 2026 17:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Mentor MUSB device controller (DCD) EP0 control-transfer state machine to better handle a race where a new SETUP arrives before the prior control transfer has fully completed (typically under high CPU load). It also adjusts MUSB port headers to avoid multiple-definition linkage issues around MUSB_BASES.

Changes:

  • Split EP0 DATA state into explicit DATA_IN / DATA_OUT states and add buffering of a “deferred” SETUP request to replay after status completion.
  • Add logic to complete pending STATUS stages before dispatching the deferred SETUP.
  • Make MUSB_BASES static const in MUSB port headers to prevent ODR/multiple-definition problems.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/portable/mentor/musb/musb_ti.h Make MUSB_BASES static const to avoid multiple definitions across translation units.
src/portable/mentor/musb/musb_max32.h Same MUSB_BASES linkage fix for the MAX32 port.
src/portable/mentor/musb/dcd_musb.c Rework EP0 control-transfer state machine and add deferred-SETUP buffering/replay to handle SETUP vs status/data completion races.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/portable/mentor/musb/dcd_musb.c Outdated
Comment thread src/portable/mentor/musb/dcd_musb.c
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 14, 2026

Size Difference Report

Because TinyUSB code size varies by port and configuration, the metrics below represent the averaged totals across all example builds.

Note: If there is no change, only one value is shown.

Changes >1% in size

file .text .bss size % diff
dcd_musb.c 2226 ➙ 2451 (+225) 171 ➙ 179 (+8) 2396 ➙ 2630 (+234) +9.8%
TOTAL 2226 ➙ 2451 (+225) 171 ➙ 179 (+8) 2396 ➙ 2630 (+234) +9.8%

Changes <1% in size

No entries.

No changes
file .text .rodata .data .bss size % diff
audio_device.c 2896 0 1259 1625 4515 +0.0%
cdc_device.c 1239 16 1092 735 1972 +0.0%
cdc_host.c 6381 487 15 985 7579 +0.0%
dcd_ch32_usbfs.c 1475 0 0 2444 3919 +0.0%
dcd_ch32_usbhs.c 1468 0 0 448 1916 +0.0%
dcd_ci_fs.c 1924 0 0 1290 3214 +0.0%
dcd_ci_hs.c 1758 0 0 1344 2535 +0.0%
dcd_da146xx.c 3067 0 0 144 3211 +0.0%
dcd_dwc2.c 4261 19 0 265 4544 +0.0%
dcd_eptri.c 2272 0 0 259 2531 +0.0%
dcd_ft9xx.c 3280 0 0 172 3452 +0.0%
dcd_khci.c 1952 0 0 1290 3242 +0.0%
dcd_lpc17_40.c 1478 0 0 648 1802 +0.0%
dcd_lpc_ip3511.c 1463 0 0 264 1683 +0.0%
dcd_mm32f327x_otg.c 1477 0 0 1290 2767 +0.0%
dcd_msp430x5xx.c 1799 0 0 176 1975 +0.0%
dcd_nrf5x.c 2940 0 0 292 3232 +0.0%
dcd_nuc120.c 1095 0 0 78 1173 +0.0%
dcd_nuc121.c 1168 0 0 101 1270 +0.0%
dcd_nuc505.c 0 0 1532 157 1689 +0.0%
dcd_rp2040.c 841 0 764 653 2258 +0.0%
dcd_rusb2.c 2917 0 0 156 3073 +0.0%
dcd_samd.c 1036 0 0 266 1302 +0.0%
dcd_samg.c 1321 0 0 72 1393 +0.0%
dcd_stm32_fsdev.c 2558 0 0 291 2849 +0.0%
dfu_device.c 777 28 712 138 914 +0.0%
dfu_rt_device.c 157 0 134 0 157 +0.0%
dwc2_common.c 603 22 0 0 615 +0.0%
ecm_rndis_device.c 1059 0 1 2759 3818 +0.0%
ehci.c 2763 0 0 6172 7701 +0.0%
fsdev_common.c 180 0 0 0 180 +0.0%
hcd_ch32_usbfs.c 2485 0 0 498 2983 +0.0%
hcd_ci_hs.c 180 0 0 0 180 +0.0%
hcd_dwc2.c 5014 25 1 513 5552 +0.0%
hcd_khci.c 2442 0 0 449 2891 +0.0%
hcd_musb.c 3070 0 0 157 3228 +0.0%
hcd_pio_usb.c 262 0 240 0 502 +0.0%
hcd_rp2040.c 2000 17 4 321 2342 +0.0%
hcd_rusb2.c 2923 0 0 245 3168 +0.0%
hcd_samd.c 2220 0 0 324 2544 +0.0%
hcd_stm32_fsdev.c 3256 0 1 420 3677 +0.0%
hid_device.c 1125 44 997 119 1244 +0.0%
hid_host.c 1240 0 0 1251 2491 +0.0%
hub.c 1384 8 8 30 1418 +0.0%
midi_device.c 1151 0 1007 624 1773 +0.0%
midi_host.c 1341 7 7 3635 4979 +0.0%
msc_device.c 2517 108 2281 806 3323 +0.0%
msc_host.c 1638 0 0 394 2033 +0.0%
mtp_device.c 1706 22 739 588 2302 +0.0%
ncm_device.c 1757 28 815 4310 6080 +0.0%
ohci.c 1919 0 0 2464 4383 +0.0%
printer_device.c 830 0 706 566 1394 +0.0%
rp2040_usb.c 378 35 631 11 1055 +0.0%
rusb2_common.c 160 0 16 0 176 +0.0%
tusb.c 450 0 383 3 452 +0.0%
tusb_fifo.c 847 0 483 0 842 +0.0%
typec_stm32.c 820 8 2 12 842 +0.0%
usbc.c 420 2 20 166 608 +0.0%
usbd.c 3522 58 91 355 3941 +0.0%
usbh.c 4668 57 111 1032 5832 +0.0%
usbtmc_device.c 2196 24 68 316 2544 +0.0%
vendor_device.c 641 0 534 565 1204 +0.0%
video_device.c 4443 5 1235 479 4914 +0.0%
TOTAL 116610 1020 15889 45167 163378 +0.0%

@github-actions
Copy link
Copy Markdown

MemBrowse Memory Report

Top 10 targets by memory change (%) (out of 2253 targets) View Project Dashboard →

target .text .rodata .data .bss total % diff
msp_exp432e401y/dfu_runtime 8,592 → 8,832 (+240) 1,048 → 1,056 (+8) 10,372 → 10,620 (+248) +2.4%
msp_exp432e401y/hid_generic_inout 9,644 → 9,884 (+240) 1,252 → 1,260 (+8) 11,628 → 11,876 (+248) +2.1%
msp_exp432e401y/hid_multiple_interface 10,428 → 10,668 (+240) 1,128 → 1,136 (+8) 12,288 → 12,536 (+248) +2.0%
msp_exp432e401y/hid_boot_interface 10,440 → 10,680 (+240) 1,128 → 1,136 (+8) 12,300 → 12,548 (+248) +2.0%
msp_exp432e401y/hid_composite 10,652 → 10,892 (+240) 1,116 → 1,124 (+8) 12,500 → 12,748 (+248) +2.0%
msp_exp432e401y/midi_test 10,748 → 10,988 (+240) 1,248 → 1,256 (+8) 12,728 → 12,976 (+248) +1.9%
ek_tm4c123gxl/dfu_runtime 8,540 → 8,780 (+240) 612 → 620 (+8) 13,316 → 13,564 (+248) +1.9%
msp_exp432e401y/cdc_dual_ports 11,592 → 11,832 (+240) 1,432 → 1,440 (+8) 13,756 → 14,004 (+248) +1.8%
msp_exp432e401y/printer_to_cdc 11,716 → 11,956 (+240) 1,412 → 1,420 (+8) 13,852 → 14,100 (+248) +1.8%
msp_exp432e401y/webusb_serial 12,096 → 12,336 (+240) 1,420 → 1,428 (+8) 14,248 → 14,496 (+248) +1.7%

Handle cases where a new SETUP arrives before the previous control transfer fully completes by buffering the SETUP and replaying it after status completion. Split EP0 DATA state into DATA_IN/DATA_OUT and finalize pending status-out completion before processing deferred SETUP.

Signed-off-by: HiFiPhile <admin@hifiphile.com>
Signed-off-by: HiFiPhile <admin@hifiphile.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants